Skip to content

Added documentation for conditional mocks #256

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 2, 2019

Conversation

adamquaile
Copy link
Contributor

Relates to #255 and php-http/mock-client#27

Copy link
Contributor

@dbu dbu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks a lot for following up on this! i like the examples and think you hit a good level of examples without being overly verbose.

i have a few changes i'd like, though.

class YourTest extends \PHPUnit_Framework_TestCase
{
/**
* @expectedException \Exception
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'd prefer dock to follow best practices and do $this->expectException(\Exception::class); in the code, on the line before sending the request.


// $requestMatcher is an instance of Http\Message\RequestMatcher

$response = $this->createMock('Psr\Http\Message\ResponseInterface');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please use use statements and the ::class pseudo constant (for the code, the comment about $requestMatcher is fine.


// $requestMatcher is an instance of Http\Message\RequestMatcher

$response = $this->createMock('Psr\Http\Message\ResponseInterface');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this not be an exception for the test to behave as expected?

@adamquaile
Copy link
Contributor Author

I'd agree with all the comments. On some of the style preferences, there's a couple of places in the docs which might want to be updated to match, for consistency if that's the preferred style.

I've updated the pull-request anyway - hope that's better.

Copy link
Contributor

@dbu dbu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks a lot!

@dbu dbu merged commit bfa9c56 into php-http:master Mar 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants